Make anyio.run_process() accept stdin argument
#859
Conversation
This is consistent with what `asyncio.create_subprocess_exec()`, `trio.run_process()`, and `subprocess.run()`, which accept such an argument. The handling of conflicts between `stdin` and `input` arguments is simplistic: the latter overrides the former without warning.
This new unit test may be able to replace the previously existing `test_run_process_connect_to_file` test, as `run_process()` internally just delegates to `open_process()`, so both are proven working by the new test.
jmehnle
force-pushed
the
run_process-accepts-stdin
branch
from
59c708d to
0452d94
Compare
There was a problem hiding this comment.
The handling of conflicts between stdin and input arguments is simplistic: the latter currently overrides the former without warning.
this feels like an unnecessary footgun with ~zero upside, I think you should add an
if stdin is not None and input is not None:
raise ValueError("only one of stdin and input is allowed"
How do the upstream APIs handle it when both are passed? |
Oh, right, this is an AnyIO only thing that we have. Never mind. |
| if stdin is not None and input is not None: | ||
| raise ValueError("only one of stdin and input is allowed") |
There was a problem hiding this comment.
you probably want a test case for this.
(maybe look over coverage config to enforce 100% patch coverage?)
There was a problem hiding this comment.
Path coverage minimum set to 100% on Coveralls. Test case added.
Changes
Make
anyio.run_process()accept astdinargument.This is consistent with what
asyncio.create_subprocess_exec(),trio.run_process(), andsubprocess.run(), which accept such an argument.The handling of conflicts between
stdinandinputarguments is simplistic: the latter currently overrides the former without warning.Checklist
tests/) added which would fail without your patchdocs/, in case of behavior changes or newfeatures)
docs/versionhistory.rst).